This is a new version of the tutorial using RAII and SLang #61
Conversation
Streamline formatting, clarify key setup steps, and consolidate redundant information in Vulkan development guide. Focus on improving readability while maintaining cross-platform support for Vulkan SDK, dependencies, and build tools like CMake and GLFW.
Refactor code to replace raw Vulkan handles with RAII wrappers from Vulkan-Hpp for better resource management and safety. Key changes include RAII usage for buffers, images, and device memory, along with modernized function calls and parameter handling. Code readability and alignment with updated Vulkan practices were also improved.
Migrated codebase to use `vk::` RAII wrappers, enhanced type safety, and updated Vulkan API conventions. Made textual corrections in documentation for grammatical consistency and clarity. Adjusted resource creation and rendering process to comply with Vulkan's multi-sampling requirements.
Corrected punctuation, grammar, and terminology for clarity and accuracy. Improved consistency in formatting, adopted RAII style for Vulkan handles, and refined examples by updating to modern Vulkan C++ bindings for better maintainability.
Fix typos and improve clarity in compute shader documentation Corrected grammatical mistakes and improved phrasing for better readability in compute shader documentation. Adjusted links to use `.adoc` extension and reformatted lines for consistency and clarity. No functional changes were made to the content. ```
Updated tutorial text to use inclusive language ("we" instead of "I") and improved readability. Modernized Vulkan instance creation using RAII and cleaner initialization code in compliance with up-to-date Vulkan best practices.
|
We might need to check if Antora supports highlighting for slang. Afair it does glsl out of the box, but for hlsl I had to manually add in a syntax highlighter. |
|
Slang isn't in the list for our antora site. Here's the list of languages we have a highlight script for. We should probably add SLang at the very least. var hljs = require('highlight.js/lib/highlight') hljs.registerLanguage('asciidoc', require('highlight.js/lib/languages/asciidoc')) |
|
We might get away by duplicating the hlsl one, and adding in a few slang keywords ;) |
|
Yep, that's what I'm thinking. I'll make a tracking issue on the vulkan-site repo and begin a pass at making a highlight.js. It might be worth it to investigate how to contribute that back upstream to help promote use of Slang? |
|
|
||
| return availableFormats[0]; | ||
| static vk::Format chooseSwapSurfaceFormat(const std::vector<vk::SurfaceFormatKHR>& availableFormats) { | ||
| return ( availableFormats[0].format == vk::Format::eUndefined ) ? vk::Format::eB8G8R8A8Unorm : availableFormats[0].format; |
There was a problem hiding this comment.
This looks very different from the original.
|
Remarks on the CMake build setup: First, kudos for providing a proper CMakeLists.txt. That has always been one of my main issues with the current tutorial 👍🏻 On Windows though it does not yet work out of the box, as you have to manually specify folders for e.g. glfw and that does not seem to work with the glfw package you can download from their site. For my own projects I moved to using CMake's FetchContent for such third party libraries, as that takes care of download, compiling and even installing these dependencies. Would it be possible to use FetchContent for the tutorial too? That would simplify project setup even further 😄 |
| * `VK_DEBUG_UTILS_MESSAGE_SEVERITY_INFO_BIT_EXT`: Informational message like the creation of a resource | ||
| * `VK_DEBUG_UTILS_MESSAGE_SEVERITY_WARNING_BIT_EXT`: Message about behavior that is not necessarily an error, but very likely a bug in your application | ||
| * `VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT`: Message about behavior that is invalid and may cause crashes | ||
| * `VK_DEBUG_UTILS_MESSAGE_SEVERITY_INFO_BIT_EXT`: Informational message |
There was a problem hiding this comment.
Should be updated to use the vulkan.hpp enums instead (e.g. vk::DebugUtilsMessageSeverityFlagBitsEXT::eWarning)
There was a problem hiding this comment.
I left it with the C version in the tutorial text on purpose with the idea that we can link directly to the spec such that the reader will know to look for the spec's version instead of the C++ version. That might be confusing though, maybe we should change it to C++ everywhere?
| } | ||
| ---- | ||
|
|
||
| The next section will introduce the first requirements that we'll check for in the `isDeviceSuitable` function. | ||
| As we'll start using more Vulkan features in the later chapters we will also extend this function to include more checks. | ||
|
|
||
| == Base device suitability checks |
There was a problem hiding this comment.
We should consider rewriting or removing this chapter all along and instead replace it with explicit device selection. This "device suitability check" has caused lots of confusion judging from community feedback.
There was a problem hiding this comment.
This might be good for after the merge to revisit. I agree, this needs to be revisited.
en/03_Drawing_a_triangle/00_Setup/04_Logical_device_and_queues.adoc
Outdated
Show resolved
Hide resolved
|
I'll have to look at the color blending again when there's more time. The cause of that change is escaping me but I'm not getting a crash when I resize. I'll have to wait until after the move when I can setup my windows machine again to test if there's something I'm missing there that might cause the resize issue. Thanks for testing @SaschaWillems |
|
Thanks for fixing. I do get a compilation error in the base code project, but that looks totally bogus and I think it's a bug with MSVC and not the actual code. All other chapters now compile fine 👍🏻 |
|
Did a build of the docs site with this PR and read through all chapters. I'm happy with the changes, and I only noticed a few minotr things like some chapters still linking to the GLSL shaders or some broken links. But those can be easily fixed after the merge. Maybe we can discuss this PR on the next docs call and decide on how to move forward. |
| []( vk::QueueFamilyProperties const & qfp ) { return qfp.queueFlags & vk::QueueFlagBits::eGraphics; } ); | ||
| // get the first index into queueFamilyProperties which supports graphics | ||
| auto graphicsQueueFamilyProperty = std::ranges::find_if( queueFamilyProperties, []( auto const & qfp ) | ||
| { return (qfp.queueFlags & vk::QueueFlagBits::eGraphics) != static_cast<vk::QueueFlags>(0); } ); |
There was a problem hiding this comment.
Maybe:
return (qfp.queueFlags & vk::QueueFlagBits::eGraphics);
There was a problem hiding this comment.
Compiler error complaining that the type is not a bool. I could cast it, but I bet the compiler would give the same asm output from casting as simply comparing against 0.
| std::vector<VkDynamicState> dynamicStates = { | ||
| VK_DYNAMIC_STATE_VIEWPORT, | ||
| VK_DYNAMIC_STATE_SCISSOR | ||
| std::vector dynamicStates = { |
There was a problem hiding this comment.
What a std::vector is this?
There was a problem hiding this comment.
clang lint has started suggesting when the compiler can figure out what the types are automatically. This is an instance of trying that suggestion out. If you feel this loses clarity to the tutorial, we can certainly be more verbose.
attachments/11_render_passes.cpp
Outdated
There was a problem hiding this comment.
Why is this file deleted?
There was a problem hiding this comment.
Afaikr because the tutorial now uses dynamic rendering, so no more need for render passes.
There was a problem hiding this comment.
Yep, same for the framebuffers. Those go away. Once we merge, I'll renumber the chapters. I also am trying to hold back a new chapter on Mobile and using Vulkan prior to 1.4 and determining when that's needed. But we're already pretty complicated PR as it stands so I don't want to add until it can be an atomic add.
attachments/13_framebuffers.cpp
Outdated
There was a problem hiding this comment.
Why is this file deleted?
Refactor device extension handling and physical device selection logic Unified device extension references to `requiredDeviceExtension` for clarity and consistency. Improved physical device selection by incorporating checks for Vulkan 1.3 features and necessary dynamic rendering capabilities. Enhanced code readability with structured initialization and modern Vulkan C++ practices.
Improved device extension handling and shader input/output structures. Enhanced readability with structured initialization. Updated device selection to validate Vulkan 1.3 features and required extensions. Refined pipeline blending and color attachment setup for clarity and correctness.
…ng up unused code
|
@SaschaWillems I'm still not getting a crash on resize for 31_compute_shader. However, the bug was in the slang shader code. It took me a bit to figure out how to correctly replace gl_PointCoord with the slang equivalent. That should work now. I'm going to see if I can test on a few more platforms and get a reproduce on the resize crashbug. |
|
Interesting. I still get that with the latest change, and even more interestingly, it does NOT happen in the swap chain code of the sample. If I add a try..catch, I do run into the catch block: try {
result = presentQueue.presentKHR(presentInfo);
if (result == vk::Result::eErrorOutOfDateKHR || result == vk::Result::eSuboptimalKHR || framebufferResized) {
framebufferResized = false;
recreateSwapChain();
}
else if (result != vk::Result::eSuccess) {
throw std::runtime_error("failed to present swap chain image!");
}
}
catch (const std::exception& e) {
throw std::runtime_error("failed to present swap chain image!");
}So maybe the raii variant of vulkan-hpp defaults to exception raising for me? Visually the sample now looks as expected 👍🏻 But anyway, it's something we can safely fix after this PR is merged ;) |
|
Also need to find out how to make this work with MSVC's intellisense. It does not find anything inside the vulkan module for me, and it seems that MSVC needs to build a special file for intellisense to work with modules: https://developercommunity.visualstudio.com/t/Additional-intermediate-build-files-for-/10642332 |
This is a rewrite of the tutorial to use SLang and RAII. While it is complete and everything is working, this is a work in progress; PR is here to enable discussion and thought.